Skip to content

Invite a friend: referral attribution across the analytics stack - #5751

Open
shai-almog wants to merge 61 commits into
masterfrom
invite-attribution
Open

Invite a friend: referral attribution across the analytics stack#5751
shai-almog wants to merge 61 commits into
masterfrom
invite-attribution

Conversation

@shai-almog

Copy link
Copy Markdown
Collaborator

Adds invite-a-friend referral attribution: mint an invite link, share it, and on
the invited device recover the invite that caused the install. Replaces what
Firebase Invites and Dynamic Links used to do, both of which have shut down.

Resolved attribution is written as persistent analytics dimensions, so every
later event — including the purchase event the framework already emits —
arrives tagged with the campaign and the referrer. Revenue and LTV per campaign
then fall out of the reports that already exist, with no new aggregation.

The server half is codenameone/BuildCloud#PENDING and is required for this to do
anything end to end.

What's here

  • com.codename1.analytics.inviteInvites, InviteRequest, Invite,
    InviteAttribution, InviteListener, the install-referrer SPI, and
    InviteButton beside ShareButton.
  • Android App Links (autoVerify) and the Play Install Referrer; iOS associated
    domains.
  • Three build hints, a PlatformFeatureCatalog entry, a developer-guide
    section, and a simulator menu for the deferred path.

Things worth a reviewer's attention

Analytics.java is not modified. resetClientId() deliberately does not
clear custom dimensions — that is right for an app's own dimensions, but the
referral ones identify an inviter, so leaving them would re-link a fresh
pseudonymous id to the same person and defeat the erasure. Rather than widening
resetClientId (which would take the app's own dimensions with it),
InviteAttributionProvider observes the client id through the init callback
Analytics already makes and erases only the cn1_* referral keys.

The package boundary is load-bearing. The catalog matches on a package
prefix, so keying one package higher would match com/codename1/analytics/Analytics
— which nearly every app references — and put the Play dependency on all of
them. That is the DatabaseConfig failure AndroidGradleBuilder.usesClass
records. Two tests pin the boundary and were confirmed to fail when the prefix
is widened.

A floor that did not exist. The plan assumed installreferrer carries a
minSdk 21 floor. Reading the actual AAR, 2.2 declares minSdkVersion 8, so no
floor is set — adding one would have dropped API 19–20 devices for nothing.

Two match types are exact and one is not. MATCH_DIRECT and
MATCH_REFERRER are exact. MATCH_FINGERPRINT is a statistical match made
server-side because the App Store carries no referrer, and it is occasionally
wrong. The docs say so and advise against paying a referral bounty on it
without disclosure.

One unrelated commit. 9a70c18 repairs a cast-semantics baseline failure
that exists on master independently of this work — #5746 renumbered an
anonymous class in AndroidImplementation from $46 to $47. Happy to split
it out.

Verification

  • Core: 6,646 tests, SpotBugs 0 findings, core-unittests verify BUILD SUCCESS
  • Android module verify BUILD SUCCESS
  • Copyright, ASCII, /// docs, no-@since, package-info, control characters,
    cast semantics, build-hint catalog (ratchet still empty) — all green
  • Vale, paragraph capitalization, guide structure, xrefs and snippet compilation
    green for the new guide section

🤖 Generated with Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d9eec6539a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
Comment thread CodenameOne/src/com/codename1/analytics/invite/InviteAttributionProvider.java Outdated
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review 🔄 Running since 2026-09-11T17:40:50.990068Z 44b972c New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Developer Guide build artifacts are available for download from this workflow run:

Developer Guide quality checks:

  • AsciiDoc linter: No issues found (report)
  • Vale: No alerts found (report)
  • Paragraph capitalization: No paragraph capitalization issues (report)
  • LanguageTool: No grammar matches (report)
  • Image references: No unused images detected (report)

@shai-almog

shai-almog commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

Compared 12 screenshots: 12 matched.
✅ JavaSE simulator integration screenshots matched stored baselines.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Cloudflare Preview

@shai-almog

shai-almog commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

Compared 151 screenshots: 151 matched.

Native Android coverage

  • 📊 Line coverage: 9.24% (9184/99417 lines covered) [HTML preview] (artifact android-coverage-report, jacocoAndroidReport/html/index.html)
    • Other counters: instruction 8.99% (47193/524920), branch 3.55% (1767/49739), complexity 3.52% (1868/53026), method 5.43% (1514/27888), class 10.89% (407/3736)
    • Lowest covered classes
      • kotlin.collections.kotlin.collections.ArraysKt___ArraysKt – 0.00% (0/6367 lines covered)
      • kotlin.collections.unsigned.kotlin.collections.unsigned.UArraysKt___UArraysKt – 0.00% (0/2384 lines covered)
      • org.jacoco.agent.rt.internal_0e20598.asm.org.jacoco.agent.rt.internal_0e20598.asm.ClassReader – 0.00% (0/1524 lines covered)
      • kotlin.collections.kotlin.collections.CollectionsKt___CollectionsKt – 0.00% (0/1187 lines covered)
      • org.jacoco.agent.rt.internal_0e20598.asm.org.jacoco.agent.rt.internal_0e20598.asm.MethodWriter – 0.00% (0/922 lines covered)
      • kotlin.sequences.kotlin.sequences.SequencesKt___SequencesKt – 0.00% (0/736 lines covered)
      • com.google.common.cache.com.google.common.cache.LocalCache$Segment – 0.00% (0/726 lines covered)
      • okio.okio.Buffer – 0.00% (0/687 lines covered)
      • kotlin.text.kotlin.text.StringsKt___StringsKt – 0.00% (0/625 lines covered)
      • org.jacoco.agent.rt.internal_0e20598.asm.org.jacoco.agent.rt.internal_0e20598.asm.Frame – 0.00% (0/570 lines covered)

✅ Native Android screenshot tests passed.

Native Android coverage

  • 📊 Line coverage: 9.24% (9184/99417 lines covered) [HTML preview] (artifact android-coverage-report, jacocoAndroidReport/html/index.html)
    • Other counters: instruction 8.99% (47193/524920), branch 3.55% (1767/49739), complexity 3.52% (1868/53026), method 5.43% (1514/27888), class 10.89% (407/3736)
    • Lowest covered classes
      • kotlin.collections.kotlin.collections.ArraysKt___ArraysKt – 0.00% (0/6367 lines covered)
      • kotlin.collections.unsigned.kotlin.collections.unsigned.UArraysKt___UArraysKt – 0.00% (0/2384 lines covered)
      • org.jacoco.agent.rt.internal_0e20598.asm.org.jacoco.agent.rt.internal_0e20598.asm.ClassReader – 0.00% (0/1524 lines covered)
      • kotlin.collections.kotlin.collections.CollectionsKt___CollectionsKt – 0.00% (0/1187 lines covered)
      • org.jacoco.agent.rt.internal_0e20598.asm.org.jacoco.agent.rt.internal_0e20598.asm.MethodWriter – 0.00% (0/922 lines covered)
      • kotlin.sequences.kotlin.sequences.SequencesKt___SequencesKt – 0.00% (0/736 lines covered)
      • com.google.common.cache.com.google.common.cache.LocalCache$Segment – 0.00% (0/726 lines covered)
      • okio.okio.Buffer – 0.00% (0/687 lines covered)
      • kotlin.text.kotlin.text.StringsKt___StringsKt – 0.00% (0/625 lines covered)
      • org.jacoco.agent.rt.internal_0e20598.asm.org.jacoco.agent.rt.internal_0e20598.asm.Frame – 0.00% (0/570 lines covered)

Benchmark Results

Detailed Performance Metrics

Metric Duration
SIMD kernel backend scalar fallback (no native SIMD)
SIMD int-add (64K x300) java 153ms / native 91ms = 1.6x speedup
SIMD float-mul (64K x300) java 133ms / native 136ms = 0.9x speedup
SIMD kernel correctness PASS (native result == scalar reference)
Base64 payload size 8192 bytes
Base64 benchmark iterations 6000
Base64 SIMD byte path gated to scalar (CPU autovectorizes scalar; explicit SIMD not beneficial here)
Base64 CN1 encode 82.000 ms
Base64 CN1 decode 69.000 ms
Base64 native encode 421.000 ms
Base64 encode ratio (CN1/native) 0.195x (80.5% faster)
Base64 native decode 331.000 ms
Base64 decode ratio (CN1/native) 0.208x (79.2% faster)
Image encode benchmark status skipped (SIMD unsupported)

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

✅ Continuous Quality Report

Test & Coverage

Static Analysis

  • SpotBugs [Report archive]
    • ByteCodeTranslator: 0 findings (no issues)
    • android: 0 findings (no issues)
    • build-hint-catalog: 0 findings (no issues)
    • build-hint-tools: 0 findings (no issues)
    • codenameone-maven-plugin: 0 findings (no issues)
    • core-unittests: 0 findings (no issues)
    • ios: 0 findings (no issues)
  • PMD: 0 findings (no issues) [Report archive]
  • Checkstyle: 0 findings (no issues) [Report archive]

Generated automatically by the PR CI workflow.

Adds com.codename1.analytics.invite: mint an invite link, share it through
the native share sheet, and on the invited device recover the invite that
caused the install. Resolved attribution is written as persistent analytics
dimensions, so every later event -- including the purchase event the
framework already emits -- carries the campaign and the referrer.

The package boundary is load-bearing, not cosmetic. The PlatformFeatureCatalog
entry that buys the Play Install Referrer library also raises the application's
minimum API level to 21, and the catalog matches on a package prefix. Keyed one
package higher it would match com/codename1/analytics/Analytics, which nearly
every application references, and put that dependency and that floor on all of
them -- the DatabaseConfig failure AndroidGradleBuilder.usesClass records, which
deleting the unused sources later does not undo. Two tests pin the boundary and
were confirmed to fail when the prefix is widened.

Analytics.java is not modified. resetClientId() does not clear custom
dimensions, which is right for an application's own dimensions but would leave
the referral dimensions behind and re-link a fresh pseudonymous id to the same
inviter. InviteAttributionProvider observes the client id through the init
callback Analytics already makes, and erases only the referral dimensions.
Adds 33 unit tests over the invite client: minting offline, url and referrer
parsing, the funnel events, the consent state machine, erasure, and
exactly-once delivery.

Three of them are the ones worth keeping honest about:

- resetClientId must clear the referral dimensions AND leave the
  application's own dimensions alone. Both halves are asserted, because
  either one alone is a bug.
- Opt-out consent mode alone must not authorise the statistical match. The
  deprecated AnalyticsService forces that mode, so the ordinary gate reports
  permission with no user choice on record.
- A dismissed share sheet must never report invite_shared, which is what
  makes the shared count a measurement rather than an assumption.

The referrer key is compared with equals and never case folded, and a test
pins that a differently cased key does not match: String.toLowerCase is
locale sensitive with no root-locale overload in this runtime, so a folded
comparison silently stops matching under a Turkish default locale.

Ten SpotBugs findings and four cast-semantics findings in the new code are
fixed rather than excluded. The one exclusion added is scoped to
Invites$InviteConnection, a one-shot ConnectionRequest that is never compared
or used as a map key -- the same idiom and reasoning as the existing
OsrmRouteService$RouteConnection entry. The casts were rewritten into the
positive instanceof form the verifier recognises, which matters beyond the
gate: ParparVM does not throw on a failed cast, so the surrounding
catch(Throwable) would never have run on iOS.
Three hints, all in the catalog rather than on the annotations, so each stays
beside the hint it composes with -- ios.associatedDomains and
android.xintent_filter are both catalog-only today, and splitting one
feature's hints across two declaration files is how they drift.

invite.domain is deliberately platform-general: both builders read it, and the
link service it names has to agree with the apple-app-site-association and
assetlinks.json served from that host.

There is no hint to turn invites on. The class scan is the switch, through the
PlatformFeatureCatalog entry -- a second source of truth for the same fact is a
second thing to keep in sync.

android.invite.signingFingerprint carries the Play App Signing warning in its
own doc text because the failure has no other surface: Google re-signs the app,
so verifying against the upload key the build holds means autoVerify fails on
every Play install, the link opens Chrome, and nothing reports an error.
Both builders now detect com.codename1.analytics.invite (and the InviteButton
that fronts it) in the class scan, and wire the platform side.

Android gets an autoVerify App Links intent filter, appended to
android.xintent_filter rather than emitted at a new manifest site. That hint is
already rendered inside the main <activity>, and rendered a second time into
the wear companion manifest, so one append reaches both and cannot drift the
way two injection sites would. It is the repo's first use of autoVerify.

The build is REFUSED when android.activity.launchMode is "standard", rather
than warned. With singleTop (the default) or singleTask a link reaches the
running activity through onNewIntent; with standard it starts a second activity
and the invite is simply lost. A warning in a build log is the thing nobody
reads, and the symptom on the device is a feature that silently never fires.

iOS appends applinks:<host> to ios.associatedDomains. The placement is
load-bearing and commented as such: the block that uncomments
CN1_HANDLE_UNIVERSAL_LINKS tests only whether that hint is non-null, so
appending one line later would leave the define commented out -- entitlement
present, handler not compiled in, every link opening Safari. The matching
associated-domains entitlement is derived from the same hint downstream, so it
is deliberately not written separately: a duplicate key fails codesigning.

Both duplicate-suppression checks compare whole delimited tokens rather than
substrings, because the failure is asymmetric and silent -- a developer's
staging entry for a longer host would otherwise read as declaring the
production one, and they would ship an app whose invite links open the browser.
Thirteen tests cover it, and the staging case was confirmed to fail against a
naive contains() implementation.
…that was never there

Adds the deterministic Android path. The link service puts cn1_invite=<code>
on the Play url, the store hands it back on first launch, and the code is
claimed verbatim -- no matching, no guessing.

The implementation is a port source excluded from the port jar's compile and
compiled inside the generated app, the mechanism ar/ai/cipher/nearby already
use, with the builder deleting the package for apps that did not ask.
Deliberately not a generated string literal like the Firebase bridge: this owns
a connection lifecycle, a reconnect path, a bounded retry and once-only
bookkeeping, and as a literal it would be invisible to review and to SpotBugs.
Registration is spliced beside the Firebase one as a direct symbol reference,
so R8 renames call site and target together and there is no keep rule to
forget.

FEATURE_NOT_SUPPORTED -- no Play Store, a sideload, another vendor's store --
is surfaced as an ordinary "no referral" answer, not an error and not silence.

The correction: the plan asserted this dependency carries a minSdk 21 floor,
and it does not. Reading the actual artifact rather than trusting the
assumption, installreferrer 2.2 (the newest release) declares minSdkVersion 8
in its own manifest. The catalog entry now sets no floor, because adding one
would have dropped API 19 and 20 devices from every invite app's Play listing
for no reason. The aar also contributes its own
BIND_GET_INSTALL_REFERRER_SERVICE permission, so none is declared here.

The package boundary still earns its keep -- it keeps the dependency and that
permission off every app that merely reports analytics.
A new Analytics chapter section covering sending, receiving, closing the
funnel, and the build wiring, with three compilable snippets.

Two things it says plainly rather than glossing:

The three match types are not equally trustworthy, and the section says which
is which. MATCH_DIRECT and MATCH_REFERRER are exact; MATCH_FINGERPRINT is a
statistical match, used because the App Store carries no referrer parameter of
its own, and it is occasionally wrong. The advice is to report it as an
estimate and not to pay a referral bounty on it without saying so.

A coarse device profile is written to local storage on first launch, before
consent, so a deferred match is still possible if consent arrives in time. The
section says so, says it is never transmitted while consent is withheld and is
deleted if consent is refused, and says why there is no alternative that also
works -- the match window closes long before a consent prompt is answered.

The Play App Signing warning is repeated here because that failure has no other
surface: verification runs against the certificate the installed APK is signed
with, which under Play App Signing is Google's key rather than the upload key,
and getting it wrong means every invite link opens the browser with nothing
reporting an error.

Vale, paragraph capitalization, guide structure, xrefs, code blocks and snippet
validation all pass, and the snippets compile.
"Send App Argument" already covers the installed-app half -- paste an invite
link into it. What it cannot reach is the deferred half, which is the one most
likely to ship broken: the install-referrer parser is otherwise exercised only
by a real Play install, on a real device, once.

The menu feeds the parser the exact string the link service puts on the Play
url, so what runs is the production path rather than a stand-in.

"Clear Invite Attribution State" exists because attribution is deliberately
once-per-install. Without it a developer can test the first-launch path exactly
once per machine, which is precisely how once-only bugs reach production.

Added to BOTH simulateMenu assembly sites. The menu is built in one place and
rebuilt from scratch in another, so an item added to only one of them silently
does not exist on the other path.
The two ports were asymmetric here, and silently so.

iOS routes every deep link through Display.setProperty("AppArg", url), which
fires Navigation.dispatchExternalUrl. Android's onNewIntent only stored the
intent, and getAppArg() then derived the value lazily through the
implementation's own setAppArg -- so setProperty never ran and the router never
fired. Anything built on @route therefore worked on iOS and did nothing on
Android. That does not surface as a bug report; it surfaces as a feature that
"just doesn't convert" on one platform.

Deliberately narrow: only ACTION_VIEW with an http or https scheme goes through
the new path. EXTRA_TEXT shares, content:// attachments and EXTRA_STREAM
payloads keep their existing lazy route. Dispatching for every intent would
double-fire against the setAppArg inside getAppArg and change behaviour for
every share-target application already in the field.

Invite attribution does not depend on this -- Invites.checkForInvite reads the
launch argument directly, which is the one path that behaves the same on both
ports, and it was written that way BECAUSE of this asymmetry. This fixes the
asymmetry itself, for everything else built on the router.
@shai-almog

shai-almog commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

Compared 181 screenshots: 181 matched.
✅ JavaScript-port screenshot tests passed.

Five review findings and fifteen PMD violations.

Consent: a restart before the user answered the prompt destroyed the deferred
profile. Analytics.addProvider synthesizes AnalyticsConsent.denied() for the
null state, and this provider is registered on every facade entry, so a second
launch before any choice arrived looking exactly like an explicit refusal --
deleting the profile captured on the first launch and moving to DECLINED, from
which a later grant could never resume. The provider now asks
Analytics.getConsent(), which returns null until a real choice is on record,
instead of believing the argument.

Outbox: entries were cleared at send time, so a registration that never landed
was never retried. The registration carries the campaign, channel, payload and
preview metadata, and a click cannot reconstruct any of it -- and the case that
lost it is the offline mint, which is the reason minting is offline at all. Each
entry is now retired by its own successful response.

flush() only drained registrations. A deferred lookup that failed because the
first launch was offline left deferredStarted set with nothing to clear it, so
the documented connectivity-recovery call silently left the attribution
unresolved until the next cold start. It now restarts the pending lookup, still
bounded by the persisted attempt counter.

Custom parameters did not survive a restart, so an answer that arrived before
the listener registered was delivered on the next launch stripped of the data
the app acts on. They are serialized into the durable record.

Invite.isRegistered() could never return true: the value is captured when the
invite is minted and registration completes asynchronously afterwards, so the
flag could only ever report what it was constructed with, contradicting its own
documentation. Removed, and replaced with Invites.isRegistered(Invite), which
reads the outbox and can actually answer.

PMD: redundant public on interface methods, six indexed loops, two missing
@OverRide. The two NonThreadSafeSingleton findings are lazy-init caches, not
singletons; they are guarded by a load flag rather than by a null check on the
field, which is both what PMD wants and more correct -- "no attribution" and
STATE_NONE are real answers, so a null check would re-read storage on every call
for the uninvited majority. No locking was added: this facade runs on the EDT.

6,649 tests pass, SpotBugs 0, PMD 0 on the invite sources.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 47a8945b01

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
Comment thread Ports/Android/src/com/codename1/impl/android/referrer/AndroidInstallReferrer.java Outdated
@shai-almog

shai-almog commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

Compared 148 screenshots: 148 matched.
✅ Native Mac screenshot tests passed.

Benchmark Results

  • VM Translation Time: 0 seconds
  • Compilation Time: 290 seconds

Detailed Performance Metrics

Metric Duration
SIMD kernel backend SSE2 (x64) / NEON (arm64) native kernels
SIMD int-add (64K x300) java 57ms / native 2ms = 28.5x speedup
SIMD float-mul (64K x300) java 54ms / native 3ms = 18.0x speedup
SIMD kernel correctness PASS (native result == scalar reference)
Base64 payload size 8192 bytes
Base64 benchmark iterations 6000
Base64 SIMD byte path active (NEON-accelerated)
Base64 CN1 encode 157.000 ms
Base64 CN1 decode 92.000 ms
Base64 native encode 679.000 ms
Base64 encode ratio (CN1/native) 0.231x (76.9% faster)
Base64 native decode 236.000 ms
Base64 decode ratio (CN1/native) 0.390x (61.0% faster)
Base64 SIMD encode 47.000 ms
Base64 encode ratio (SIMD/CN1) 0.299x (70.1% faster)
Base64 SIMD decode 44.000 ms
Base64 decode ratio (SIMD/CN1) 0.478x (52.2% faster)
Base64 encode ratio (SIMD/native) 0.069x (93.1% faster)
Base64 decode ratio (SIMD/native) 0.186x (81.4% faster)
Image encode benchmark iterations 100
Image createMask (SIMD off) 7.000 ms
Image createMask (SIMD on) 3.000 ms
Image createMask ratio (SIMD on/off) 0.429x (57.1% faster)
Image applyMask (SIMD off) 36.000 ms
Image applyMask (SIMD on) 27.000 ms
Image applyMask ratio (SIMD on/off) 0.750x (25.0% faster)
Image modifyAlpha (SIMD off) 27.000 ms
Image modifyAlpha (SIMD on) 21.000 ms
Image modifyAlpha ratio (SIMD on/off) 0.778x (22.2% faster)
Image modifyAlpha removeColor (SIMD off) 26.000 ms
Image modifyAlpha removeColor (SIMD on) 21.000 ms
Image modifyAlpha removeColor ratio (SIMD on/off) 0.808x (19.2% faster)

…r read

Six findings on this PR, all valid.

The client never read invite.domain. The builders generate the Android intent
filter and the iOS associated domain from that hint, but getLinkBase() only ever
consulted cloudServerURL and the default -- so an app that set a custom host
minted links for cloud.codenameone.com while its own app-links registration
named something else, and the installed app never opened its own links with
nothing reporting an error. Both builders now stamp the resolved host into the
app and the client reads it, so the two cannot disagree.

The deferred profile was written before the consent check. pendingRecord()
persists on the spot and onConsentChanged only deletes a record that already
exists when it runs, so a user who had ALREADY refused got a profile written on
their next launch and it stayed indefinitely -- contradicting the documented
promise that a refused profile is deleted. An explicit refusal now writes
nothing at all. An unset choice still captures, which is the point: the match
window closes long before a prompt is answered.

A terminal no-match was not durable. resolved:false only updated memory, so
loadState() resurrected the lookup on every launch and an ordinary uninvited
install re-queried the server and re-fired attributionUnavailable for ever.

Storage.writeObject's result was ignored. Storage was chosen over Preferences
precisely because it reports a failed write; deleting the pending record after
one left neither an attribution nor any retry information.

Re-attribution left stale dimensions: a later invite with no campaign kept the
previous one, so events carried the new code beside the old campaign.

A transient Play Store failure burned the once-only flag, so a later flush
skipped the deterministic referrer for ever and fell back to a guess. Only
terminal outcomes are recorded now.

One test failed and deserved to. It used AnalyticsConsent.none() to mean "not
decided yet", but none() is an explicit refusal; the fix exposed that the test
encoded the wrong semantics. Split into undecided (null) and refused.

Vale caught what a narrower local run did not: the build hint doc strings are
rendered into the generated guide table and linted there. Fixed at source; the
whole guide is clean across 123 files.

6,650 tests pass, SpotBugs 0.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3c43f5c2bf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
Comment thread CodenameOne/src/com/codename1/analytics/invite/InviteStore.java Outdated
Four findings, all valid.

A response already on the wire could undo a privacy operation. When consent is
withdrawn or resetClientId() runs, both delete the pending record and clear the
referral dimensions -- but the claim or match request they raced still arrived,
and resolve() wrote the attribution and dimensions straight back under the fresh
identity. Every lookup now carries the epoch it was issued under and a response
whose epoch no longer matches is dropped, with the permission re-checked as
well. Two tests cover it.

The direct-link path had no denial guard. The earlier fix put one in
beginDeferred(), but checkForInvite() treats a consumed URL as handled and skips
that entirely -- so a refused user opening an invite link still had a profile
persisted, by the other route. Same guard, both entry points.

The outbox cap silently discarded unacknowledged registrations. Once entries
were retired on acknowledgement rather than at send time, evicting the oldest
became a way to lose an invite whose link had already been shared: the code
carries no inviter, campaign, payload or parameters, so a later click can never
be joined to any of it. The ceiling is now 512 rather than 32, and breaching it
is logged rather than silent. I am keeping a ceiling -- an unbounded on-device
queue is not something to ship -- but it is now far outside anything the design
contemplates.

The Android filter claimed every invite link on the shared domain. This is the
Android twin of the apple-app-site-association collision the slug already solves
on iOS: a bare /i/ prefix makes every invite-enabled app an eligible handler for
every invite url, so Android shows a chooser or opens the wrong app, and the
slug inside the path cannot disambiguate because the filter accepts them all.
The new invite.slug hint scopes it to /i/<slug>/. Without a slug the broad
filter is still emitted and the hint documents why -- a filter matching nothing
would be worse.

6,652 tests pass, SpotBugs 0, the whole guide is Vale-clean.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e63dfeddcc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java
Comment thread CodenameOne/src/com/codename1/analytics/invite/InviteStore.java Outdated
A non-2xx response reached postResponse() exactly as a 200 did --
ConnectionRequest reads error bodies by default and the error path falls
through -- so a transient 5xx retired the durable registration as though the
server had accepted it, and an error body parsed as "not resolved" turned one
bad minute upstream into a permanent "you were not invited". Gate both on the
status.

The build scoped the Android filter and the iOS path claim to /i/<slug>/ but
only stamped invite.domain into the app, so the client learned the slug from
the link service -- which the first invite is minted before ever reaching. That
first link could not match the build's own filter. Stamp the slug too, and let
it outrank the stored value.

A terminal no-match deleted the pending record, and an absent record reads back
as STATE_NONE: the next launch built a fresh profile and asked again, for ever.
Replace it with a marker that carries the state and nothing else -- durable, and
holding none of the profile, which existed to be matched and now has nothing to
match against.

Under re-attribution a pending claim lost to the older resolved attribution in
loadState(), so a claim interrupted by process death was never retried and last
touch silently kept losing to first. Consult the pending record first, and only
under re-attribution: without it a stale record must never reopen a settled
attribution.

The manifest filter was suppressed by any existing filter naming the host, so an
app already routing cloud.codenameone.com/account/ never got one and its invite
links kept opening the browser. Require the path too, and accept only a prefix
that really covers /i/<slug>/.

InviteStore.writeOutbox discarded writeObject's result, so a full store lost the
campaign, channel, payload and preview of a link already handed out with no
sign. Propagate it and send that one registration immediately instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bfe54295f7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Ports/Android/src/com/codename1/impl/android/referrer/AndroidInstallReferrer.java Outdated
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java
A Play Install Referrer outage is not an answer. Two failed connection attempts
left the source's once-only flag deliberately unset so a later launch could read
the exact referrer, and then the statistical fallback's no-match settled the
install as organic anyway -- throwing away a deterministic result that was still
reachable. A transient failure now marks the record, and a no-match against that
mark stays pending, bounded by the attempt cap and the window as before.

setAttributionWindow(0) recorded nothing: setState() only rewrites a record that
exists, and on a fresh install none does, so the listener heard "unsupported" on
every launch. It writes the terminal marker now -- the one marker that carries a
reason, because it is the only terminal answer that can stop being true, and an
application that later ships a non-zero window is asking for attribution again.

The filter check searched the whole hint value, so a filter for our host on
/account/ and an unrelated host on /i/ claimed coverage between them although
neither would ever open an invite link. Host and path are matched within one
<intent-filter> now.

isRegistered() read absence from the outbox as acknowledgement, which is exactly
wrong for the registration sent directly because the outbox could not be written:
never queued, so the queue says nothing about it. Those codes are tracked in
memory until the server acknowledges them -- in memory because the durable store
is the thing that failed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5610439922

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java
…al one

A deferred lookup already on the wire ran under the same epoch as the direct
claim that superseded it, so both answers passed the guard and a statistical
match arriving second overwrote the exact one -- its dimensions and its durable
record with it. The direct claim advances the epoch, which is how every other
supersede in this class is expressed.

The pending branch added last round still called attributionUnavailable(), which
is the terminal callback: it says no invite will be attributed, and it sets
deliveredThisRun, so a referrer that succeeded moments later in the same process
could no longer deliver inviteReceived() -- while a relaunch could deliver it as
a second outcome after the first said never. A pending outcome now tells the
listener nothing.

Refusing consent deleted the pending record and then called setState(), which
has nothing to rewrite once the record is gone, so STATE_DECLINED lived in memory
and the listener was told again on every launch. It writes the profile-free
marker instead, at all three refusal sites. The marker carries its reason, and
beginDeferred reopens it when the reason stops being true -- a granted consent
here, a re-enabled window for the other one -- read from the condition itself
rather than from a second stored copy of it.

A successful referrer read carrying no invite is definitive, and it left an
earlier outage's referrerRetry marker in place, so the following no-match looked
retryable and every launch asked again until the attempt cap. The non-retryable
path clears it.

Two consent tests asserted the record was absent, for a promise that is about the
profile. They assert the profile fields are gone now, which is the property the
documentation actually makes and the only one that can survive a relaunch.

Also fixes the PMD NonThreadSafeSingleton that build-test (8) caught in
loadState: the record is reduced to a value before the branch, so there is no
null-check-then-static-assign shape. Not a lock -- this facade runs on the EDT
and adding one would be the real mistake.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 57fc4c36a5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java
Two review findings, plus two defects of my own found while verifying them.

reset() threw away resetVerified()'s answer. The detection added last round
was real and then dropped at the one call site an application reaches, so a
PENDING record that outlived its erasure was still claimed on the next
launch and a surviving outbox entry still went out under the old client id.

It latches the gate now -- but only when something really did survive.
resetVerified() also answers false for a reason that leaves nothing behind,
no Storage at all, and latching on that would block a device with no invite
data to block over: nothing else proceeds until an erasure succeeds, and
that is a severe consequence to hang on a device state.

The mirror image was also wrong, and was mine from the previous commit:
only eraseInternal() ever cleared the flag, so a plain reset() that
SUCCEEDED left a stale latch standing, and the next gated call then ran a
full erasure -- tombstone included -- turning an ordinary reset() into a
terminal state the application never asked for. A successful reset clears
it, which is what the flag has always claimed to mean.

The tap time is kept now. An App Clip invocation is resolved by iOS from
the association file, so it never reaches our redirect: the clip is the
only witness, and the native side clears the handoff as it reads it.
Dropped in the callback it was gone, and getClickTimestamp() answered zero
for every App Clip attribution. It is persisted in the pending record --
the claim can fail and be resent from there -- and sent as clickedMillis.

The Play referrer had the identical bug and the finding did not mention it:
onReferrer carries clickSeconds and nothing read it either. Fixed together,
because fixing one platform and not the other leaves the two reporting the
same field differently, which is worse than both being wrong.

Asserted as a bare JSON number rather than a quoted one, because the server
binds it to a long: a string coerces today and stops the moment anything
there gets stricter.

And the PMD gate caught five violations I had pushed: PATH_MATCH left
behind by the deleted /match endpoint, and four missing @OverRide on the
App Clip callback, which the install-referrer callback directly above it
has. I had been skipping the static-analysis gates locally because SpotBugs
will not run under this JDK; PMD does, through pmd:pmd, and it is clean now
-- verified by injecting an unused field and confirming it is reported,
because a report that is empty because nothing was analysed looks exactly
like one that is empty because nothing is wrong.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d2383285ea

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CodenameOne/src/com/codename1/analytics/Analytics.java
…later

resetClientId() clears the reserved cn1_ dimensions, and Preferences.set
discards its own write-failure boolean -- the same trap Continuity documents
and the reason Invites uses Storage instead. So on a full or read-only
store the entries vanished from the in-memory map and stayed in the file,
and the next launch loaded them back and attached the referral identity the
user asked to be rid of to their NEW client id. One launch later, with
nothing in memory left to notice.

Two halves, because each closes a case the other cannot.

persistDimensions() reports now, by reading the value back rather than
trusting the write, and clearReservedDimensions() retries once on failure.
That closes it inside the process, where the cause is usually transient.

Across a restart no in-memory retry survives, so the persisted blob is
stamped with the client id it was written under. loadDimensions() drops
reserved entries whose stamp names an identity that has since been reset --
the erasure finishing late -- and keeps the APPLICATION's own dimensions,
because those are not what an erasure asked about and losing a plan or role
the app set would be a second bug in the name of fixing the first. An
absent stamp reads as current, so a file written before this existed is not
discarded.

Both directions are pinned: a stamp from an erased identity drops only the
cn1_ entries, and a stamp from the CURRENT identity keeps them -- without
that second test the drop could be keyed on the prefix alone and throw the
referral away on every ordinary launch. The first was revert-probed: with
the check disabled the test reports the erased campaign coming back as
"spring", which is the bug exactly.
AnalyticsFacadeTest had no copyright header, and the gate checks every file
a change TOUCHES rather than only the ones it adds -- so editing it made the
missing header this branch's problem. It carries the Codename One GPLv2 +
Classpath header now, the same one every other file in this package has.

Mine to have caught before pushing: I ran the header gate earlier in the
branch and did not re-run it after the commit that touched this file, which
is exactly the case it exists for.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 51173b4bc5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CodenameOne/src/com/codename1/analytics/Analytics.java Outdated
…raps

The verification from the previous commit was worthless and I should have
read Preferences before writing it. Preferences.set updates a static
Hashtable and Preferences.get reads that same Hashtable, so comparing a
value with what comes back compares memory with memory: it reports success
for a write that never reached the disk.

It was worse than useless. persistDimensions() stamped the file with the
NEW client id in the same breath, so a failed erasure left the OLD reserved
dimensions sitting under a stamp that claimed them as current -- and the
stamp, which is the mechanism that was supposed to catch exactly this, said
they belonged. The fix made the bug harder to see.

The check is gone, with a note saying why it cannot work, and the stamp now
carries the whole job. An ABSENT stamp counts as foreign rather than
current, which is the difference between a mechanism that works and one
that works only when the write it depends on succeeded: on a device whose
file predates the stamp, or where the same storage failure that broke the
erasure also stopped the stamp landing, there is nothing to compare. The
trade is explicit -- a reserved dimension dropped wrongly is rewritten by
the next attribution; an erased identity coming back is not recoverable --
and clientId() is used rather than the field, because loading can happen
before the id is materialised and a null made every file look current.

ios.invite.appClip=false disabled the receiving side too. It means "do not
GENERATE a clip", which is what a developer sets when they ship one of
their own -- and it was suppressing the app group, the native define and
the registration of IOSAppClipHandoff along with it, so a custom clip wrote
the documented handoff into the documented container and nothing read it.
Generation and reception are separate questions now, and the hint's
documentation said the wrong thing too.

The clip also hard-coded TARGETED_DEVICE_FAMILY=1, on the belief that App
Clips do not run on iPad. They do -- and an ios.project_type=ipad build has
an iPad-only app target, so an iPhone-only clip inside it shares no family
with its container and App Store validation rejects the archive. It uses
the same host-family helper every other embedded target here uses.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e2d3dc661b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
InviteStore.delete() overwrites a record it could not remove with an empty
one, deliberately: an empty record carries no code, no inviter and no
campaign, so a delete that cannot happen at least leaves nothing behind.
But the state read defaulted an absent "state" key to STATE_PENDING, so
that tombstone came back as a pending lookup -- and under re-attribution a
pending state outranks the durable attribution, so the settled claim was
resubmitted and invite_install or invite_opened counted one install twice.
An empty record reads as absent now.

And the class documentation still told applications that this feature
writes a coarse device profile -- OS version, hardware model, language,
screen size -- to local storage before consent. It has not since App Clips
replaced the statistical match: pendingRecord() stores timing and state,
and the code it keeps is one the person produced by tapping an invite.

That is worse than a stale comment. It is the paragraph a developer copies
into their own privacy disclosure, so leaving it there publishes a claim
about data collection that does not happen -- and it would reasonably put
someone off the feature entirely. It now says what is actually stored.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6d7a38f592

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
delete() falls back to overwriting a record it cannot remove with an empty
one, and the previous commit stopped that empty record reading as pending.
This is the case where BOTH fail: the real pending state survives beside
the new attribution, and under re-attribution loadState() prefers it --
deliberately, so a claim interrupted by process death is retried. The
already-successful claim was therefore resubmitted on the next launch, and
a second invite_install or invite_opened was emitted for one install.

The record is overwritten with the terminal state when the delete fails.
That says what the deletion would have said, in a record the store has just
proved it will not remove, and it carries no code and no inviter -- so if
that write fails too, what is left is the record that was already there and
nothing new is disclosed. The failure is logged rather than assumed away.

Revert-probed: with the check removed the test reports the settled install
coming back as pending, which is the resubmission exactly.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fb39ecfc8e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

ios.invite.buildSettings.PRODUCT_NAME can override what the clip is built
as, but the embed reference looked for CN1InviteClip.app in
BUILT_PRODUCTS_DIR regardless -- so such a build failed while copying a
product that was never produced.

It goes through effectiveExtensionProductName, the same helper the VPN
tunnel and Matter targets use, which also refuses a value this build cannot
evaluate rather than emitting a reference that cannot resolve: an Xcode
condition is legal in that setting and nothing here can expand it, so the
honest answer is to say which hint is unusable and why.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7066438f03

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java
Outbox entries leave the queue only when their OWN response acknowledges
them -- which is right, because the campaign, channel, payload and preview
cannot be reconstructed from a click -- but that leaves an entry drainable
while its request is still outstanding. create() flushes unconditionally,
so N invites minted in a burst sent N(N+1)/2 requests: six produced
twenty-one, and the 512-entry cap puts a full queue past 131,000.

Entries now carry an in-flight mark, and the two flushes are told apart.
create()'s own flush skips what is already going out; the PUBLIC flush()
does not, because it is documented as the "I have just regained
connectivity" call and its whole job is resending a request that went out
over a dead network and will never answer. An existing test pins that
second behaviour and caught the first attempt, which suppressed both.

The mark is released on every outcome, including handleException -- where
postResponse() never runs. Without that a transport failure left the entry
marked for the life of the process and no later drain retried it, trading
an amplification bug for a lost registration, which is the worse of the
two. It is not persisted, so a process that dies with requests outstanding
retries them on the next launch.

And ios.invite.universalLinks=false disabled the receiving side, exactly as
ios.invite.appClip=false did before it. It means "do not inject the
associated domain, I manage the entitlement myself" -- and it was also
suppressing the app group, the native define and the registration of
IOSAppClipHandoff, so an app that had configured its own domains correctly
had nothing reading the handoff and every iOS install settled as no_match.
Each hint is applied where the thing it governs is done, and neither gates
the machinery any more.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4e39d4f873

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CodenameOne/src/com/codename1/analytics/invite/InviteAttributionProvider.java Outdated
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
All three are the same shape, and it is the shape Preferences forces:
`set` updates a static table and swallows the store's answer, so no write
to it can be verified. Everything durable that CAN report -- the three
InviteStore records -- already does. These are the places that trusted the
other kind.

reset() cleared the referral dimensions in memory and asked Preferences to
persist that, while resetVerified() reported success on the records it can
verify. A plain reset keeps the same client id, so the owner stamp still
matched and the next launch loaded the old cn1_invite* values back and
transmitted them, despite reset() promising to forget them.

The attribution record is the authority and it IS verifiable, so the
dimensions are reconciled against it once per process: if no record stands
behind them, they are the stale copy and the erasure finishes on the next
launch instead. That is the best an unverifiable store allows, and it is
self-healing rather than dependent on the failing write ever succeeding.

The provider's identity baseline had the mirror problem. A failed baseline
write leaves the same empty value a first registration does -- so the next
resetClientId() in that process read the new id as its first baseline,
skipped eraseInternal(), and left the old attribution and the queued
registrations attached to the identity just reset. Records with no baseline
are treated as the erasure that never completed; a device with no records
is the genuine first registration it looks like.

And when both the PENDING delete and its settled-marker replacement failed,
the held fallback was discarded anyway -- committing the resolution with a
durable STATE_PENDING on the disk, which re-attribution prefers, so the
next launch resubmitted a claim that had already succeeded. The fallback is
kept when its own write failed, so the next read retries it.

The first is revert-probed: without the reconciliation the test reports the
erased campaign still reading "spring".

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 23ef7ab60c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Ports/iOSPort/nativeSources/CN1InviteAppClip.m
CN1InviteAppClip.m put both native implementations behind
CN1_INCLUDE_INVITE_APPCLIP, but IOSNative.java declares the methods
unconditionally -- and ParparVM needs a symbol for every native declaration
whether or not anything reaches it. So an ordinary iOS application that
never heard of invites would have failed to LINK, on a feature it does not
use, which is the worst possible place for this to be felt.

I reasoned that dead-code elimination would drop the unreferenced Java
methods along with the class nothing registers. That was wrong, and the
file next door says so in as many words: CN1WebAuthn.m supplies #else stubs
for exactly this reason and explains it. This now does the same, answering
what a device with no clip answers anyway, so nothing depends on which
branch compiled. Verified by compiling the file with the define OFF.

SERVICE_DISCONNECTED arriving as a RESPONSE CODE fell into the terminal
default, which records PREF_ATTEMPTED and refuses another read for ever. It
is the same transient state the disconnect callback reports, and handling
that callback -- as this branch already does -- does not cover this path:
an invited install whose exact Play referrer was still available on the
next connection settled permanently as organic. It takes the transient
route now, so the once-only flag stays unburnt.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cd49dddb92

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

… upload

ASSETCATALOG_COMPILER_APPICON_NAME was blanked on the clip target. An App
Clip is a full application bundle and App Store validation rejects one with
no icon, so every invite-enabled iOS archive would have been refused at
upload -- for a target the developer never asked to maintain and cannot
fix from their own sources.

The host's Images.xcassets is copied into the clip rather than a
placeholder generated: a clip card showing a different icon from the app it
installs is its own confusion, and the person seeing it has installed
nothing yet. appendFilesToXcodeProjGroup already adds an .xcassets
directory as a single resource -- it has to, or Xcode fails with "Multiple
commands produce Contents.json" -- so staging it is all that is needed. A
build with no host catalog says so rather than naming a catalog that is not
there, which would fail the build instead of the upload.

The derived product name went into a single-quoted Ruby literal unescaped,
so a legal PRODUCT_NAME containing an apostrophe -- "Friend's Clip" --
broke fix_xcode_schemes.rb and the iOS build with it. It goes through
escapeRuby like every neighbouring target.

And the ios.invite.appClip documentation still said it was ignored when
ios.invite.universalLinks is false. That stopped being true when the two
hints were separated: universalLinks now means "I manage the domains
myself" and leaves the clip, the app group and the reader in place, so a
developer relying on the old wording would get a second target and its
signing requirements unannounced. appClip=false is the only thing that
suppresses generation, and the entry says so.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a4fb34201e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
markTerminal() copies the code provenance into the reopenable DECLINED
marker so a user who refuses consent when the link arrives and grants it
afterwards keeps their exact claim. codeClicked was added to the record
later and never added to that list, so a withdraw-then-grant cycle resent
the claim with a zero time -- and for an App Clip that time is
irrecoverable, because the invocation never reaches the redirect and the
clip cleared its own copy as it was read.

The clip handoff also charged the retry budget twice: the local read bumped
attempts and the claim it leads to bumped them again, so the first network
claim started at 2 and the install settled terminal after four requests
instead of the five MAX_ATTEMPTS promises. The install-referrer path never
bumped there, so the clip path was the inconsistent one; both now spend the
budget only on network attempts, and a source that answers nothing at all
is bounded by the attribution window on both.

And the SpotBugs finding CI caught: a redundant null check on
Analytics.getDimensions(), which returns a fresh copy and never null.

That gate has been blind on my side all branch -- SpotBugs will not run
under the JDK 8 toolchain, so I had been passing -Dspotbugs.skip=true. It
runs under JAVA17_HOME, and the recipe needs stating because two earlier
attempts reported clean without running at all: the report has to be
DELETED first, and the run needs network access or checkstyle fails in the
validate phase and spotbugs never executes, leaving the previous report to
be read as success.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 801bfedd88

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java
Comment thread Ports/Android/src/com/codename1/impl/android/referrer/AndroidInstallReferrer.java Outdated
…leased

erasurePending is a static, so a reset() whose deletes failed and whose
application then exited left nothing to retry from -- and a plain reset
keeps the client id, so the provider sees no identity change on the next
launch and does not erase either. The surviving attribution came back and
was transmitted, which is the one thing reset() promises cannot happen.

There is a durable ERASURE record now, picked up once per process before
anything can read or transmit. It is a WRITE recording a failure to DELETE,
which is deliberate: a store refusing removals may still accept a small
write, and if it refuses that too this is no worse than what came before.
Revert-probed -- without the resume the test reports the erased attribution
coming back.

The in-flight mark was released by a callback that never runs. These
requests are fail-silent, and NetworkManager's fail-silent branch only logs
-- it never calls handleIOException or handleRuntimeException, so nothing
reaches the request's own hooks. A transport failure left the entry marked
for the life of the process and every automatic drain skipped it, trading
an amplification bug for a registration only an explicit flush() or a
restart would resend. It is a 60-second time bound now: a burst happens in
milliseconds, so the bound serves the original purpose completely while
guaranteeing the queue heals, and expired marks are dropped as they are
read.

The Play referrer's one-shot flag was burnt before the code was handed
over, so a process killed in between lost the exact referrer for ever and
the next launch settled the invited install as no-match. The handoff comes
first now.

Being precise about what that buys: Invites marshals onto the EDT, so a
callback arriving on a binder thread has its persist QUEUED rather than
done. The window goes from always to the callSerially latency, not to zero.
Closing it completely would need the port to know what the framework did
with the value, which the SPI deliberately does not tell it.

And DM_NUMBER_CTOR on the new map -- new Long() where Long.valueOf()
belongs. Caught by running SpotBugs locally this time rather than by CI.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aa67da38b2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java Outdated
Comment thread CodenameOne/src/com/codename1/analytics/Analytics.java
Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
…dimensions

Three review findings, each a case the code got wrong in a way nothing
reports.

The warm-link path stored a data-less COPY of the intent. That fixed one
reader -- an onNewIntent() override reading the intent it was handed -- and
broke two others: the documented `android.intent.data` property is published
from whatever the activity has stored, and native integrations read
getActivity().getIntent().getData(). Both saw a warm deep link as no deep
link at all while cold links still carried it. The intent is now stored
unmodified and the url is marked delivered by remembering the intent's
identity, which suppresses only getAppArg()'s second delivery.

Dimension files with no owner stamp are adopted rather than dropped. An
absent stamp means the file predates the stamp -- persistDimensions() writes
both keys into one preferences record -- and back then setDimension()
reserved no prefix and the framework wrote no `cn1_` dimension, so anything
with that prefix in such a file is the application's own and dropping it
deleted segmentation from an app that never asked for an erasure. A stamp
that is present and different is still foreign. The reserved prefix is now
documented on setDimension() rather than only on resetClientId().

abandonReplacement() verifies the deletion, like the resolved path already
did. Ignoring it left the replacement's PENDING record on disk while memory
moved on to RESOLVED, and loadState() prefers a surviving pending record --
so a claim that had already ended definitively was resubmitted every launch,
for ever. The second, open-coded copy of that abandonment now calls it.

Both behaviour changes are covered by tests verified against the unfixed
code; the unused `pending` parameter PMD flagged is gone with it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c67db7d1f9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java
beginDeferred() runs at most once per process, which is right for the first
attempt and wrong for every later one. The lookup is fail-silent, so a
request that never answered leaves deferredStarted set with nothing to clear
it, and a claim answered with "retry" -- the ordinary state of an invite
minted offline, whose registration has not landed yet -- is pending with no
attempt outstanding. Either way the documented call-me-from-start() contract
did nothing for the rest of the run: the invite resolved on the next cold
start, after an onboarding that could have had its payload, its callback and
its dimensions.

checkForInvite() now re-arms a pending lookup with nothing in flight, which
is what flush() already did for the regained-connectivity case. Bounded by
lookupInFlight(), so an application that calls it from every form cannot
spend the attempt budget faster than one attempt per retry interval, and by
the persisted cap and the attribution window beyond that. The contract is
documented on the method rather than left to be discovered.

The plugin's source-level test asserted the intent-copy shape that the
previous commit replaced, and failed build-test (8) and build-linux-jdk8 on
exactly that. It now asserts what replaced it: the stored intent keeps its
data, the url is marked delivered by identity, and getAppArg() honours the
mark.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b19ff95252

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CodenameOne/src/com/codename1/analytics/invite/Invites.java Outdated
reconcileDimensions() only ever reconciled one way. It dropped reserved
dimensions with no record behind them, and accepted whatever the dimensions
said whenever a record existed -- so it never noticed the opposite failure.

Preferences.set swallows its write failure, so a resolve can commit the
attribution and fail to persist the four dimensions: correct in memory for
the rest of that process, and gone on the next launch. Every later batch then
carried no campaign at all, or -- under re-attribution, where the previous
invite's values are still on the disk -- the campaign the install no longer
belonged to, crediting its revenue to the wrong cohort. Nothing looked again,
because the only thing that could have was satisfied by the record existing.

The record is the half that can report whether it was written, so it is the
authority: when the persisted dimensions disagree with it they are rewritten
from it. Compared first, so an ordinary launch does not pay for a storage
write it has no use for. Verified against the unfixed code, where the test
sees the previous campaign survive a resolve that replaced it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant